Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(compat): update patch for typescript 4.4 #3297

Merged
merged 10 commits into from
Aug 31, 2021
Merged

feat(compat): update patch for typescript 4.4 #3297

merged 10 commits into from
Aug 31, 2021

Conversation

merceyz
Copy link
Member

@merceyz merceyz commented Aug 15, 2021 •
edited
Loading

What's the problem this PR addresses?

  • The PnP patch for TypeScript doesn't apply to typescript@>=4.4.1-rc
  • To quote the test file:
The PnP patch returned the packageLocation with a trailing `/` which
caused TypeScript to calculate the `subModuleName` incorrectly[1].
This caused different files where only the first character of their
basename was different to end up with the same packageIdKey[2].
In this test the affected files were
@types/lodash-es/find.d.ts -> @types/lodash-es/[email protected]
@types/lodash-es/bind.d.ts -> @types/lodash-es/[email protected]

1: https://github.com/microsoft/TypeScript/blob/98866a5a740b487c20046d4ffaa36aa1f202dde9/src/compiler/moduleNameResolver.ts#L20
2: https://github.com/arcanis/TypeScript/blob/36225c32609137d29008e9d3ecf48c6f51456eb5/src/compiler/program.ts#L2625
  • The references patch doesn't preserve paths when resolving virtuals and assumes everything points to a package root

Fixes #3058
Fixes #3231
Fixes #3117
Fixes #3350
Fixes #3262
Closes #3275
Closes arcanis/TypeScript#10

How did you fix it?

Checklist

  • I have read the Contributing Guide.
  • I have set the packages that need to be released for my changes to be effective.
  • I will check that all automated PR checks pass before the PR gets reviewed.

@merceyz merceyz requested a review from arcanis August 15, 2021 22:31
@merceyz merceyz changed the title feat(compat): update patch for typescript 4.4 fix(compat): update patch for typescript 4.4 Aug 15, 2021
@merceyz merceyz changed the title fix(compat): update patch for typescript 4.4 feat(compat): update patch for typescript 4.4 Aug 15, 2021
This was referenced Aug 16, 2021
Closed
Closed
@merceyz
Copy link
Member Author

merceyz commented Aug 19, 2021

Failing CI will be fixed by #3315

@merceyz merceyz mentioned this pull request Aug 19, 2021
Closed
1 task
@merceyz merceyz force-pushed the merceyz/fix/typescript branch from eba97bc to eb70ab5 Compare August 23, 2021 18:12
@merceyz merceyz mentioned this pull request Aug 25, 2021
Closed
2 tasks
@merceyz merceyz force-pushed the merceyz/fix/typescript branch from 40934d5 to 5b5e0c8 Compare August 25, 2021 12:07
@merceyz merceyz force-pushed the merceyz/fix/typescript branch from 5b5e0c8 to f88cc7c Compare August 25, 2021 12:30
@sushruth
Copy link

Looking forward to this fix! this helps many projects based on nx.

@merceyz merceyz mentioned this pull request Aug 26, 2021
Closed
1 task
@merceyz
chore: update to 4.4.2
00486eb 
There was no conflict but updated the patch either way
cmfcmf added a commit to roc-and-roll/roc-and-roll that referenced this pull request Aug 28, 2021
@cmfcmf
Update yarn to PR #3297
903ca14 
yarnpkg/berry#3297
@arcanis arcanis merged commit ffe1a64 into master Aug 31, 2021
@arcanis arcanis deleted the merceyz/fix/typescript branch August 31, 2021 11:28
@SimenB
Copy link

SimenB commented Sep 1, 2021

Will this be backported to v2? I have a bunch of projects which still supports node 10

This was referenced Sep 1, 2021
Closed
Closed
@merceyz merceyz mentioned this pull request Sep 1, 2021
Closed
1 task
@sushruth
Copy link

sushruth commented Sep 2, 2021

This may have broken some base scenarios - https://github.com/sushruth/simple

here is a simple repro repo which imports react and console logs it. TSC finds that erroneous like so -

image

@merceyz
Copy link
Member Author

merceyz commented Sep 2, 2021

It didn't, see #3350 (comment)

@sushruth
Copy link

sushruth commented Sep 2, 2021

@merceyz still fails. It fails for all packages which have typings included too. Added immer to show this -
image

@merceyz
Copy link
Member Author

merceyz commented Sep 2, 2021

Read the rest of the comments in the issue I linked please

@sushruth
Copy link

sushruth commented Sep 3, 2021

@merceyz my bad. Thanks!

arcanis pushed a commit that referenced this pull request Sep 3, 2021
@merceyz @arcanis
feat(compat): update patch for typescript 4.4 (#3297)
51a2069 
* fix(compat): remove trailing slash

* fix(compat): update patch for 4.4

* test: add testcase

* chore: versions

* fix: handle references that doesn't point to a package root

* fix: backport references fix to 4.2

* chore: dedupe

* fix: avoid direct requires to `pnpapi`

* chore: reuse files in validation

* chore: update to `4.4.2`

There was no conflict but updated the patch either way
arcanis pushed a commit that referenced this pull request Sep 3, 2021
@merceyz @arcanis
feat(compat): update patch for typescript 4.4 (#3297)
69f195a 
* fix(compat): remove trailing slash

* fix(compat): update patch for 4.4

* test: add testcase

* chore: versions

* fix: handle references that doesn't point to a package root

* fix: backport references fix to 4.2

* chore: dedupe

* fix: avoid direct requires to `pnpapi`

* chore: reuse files in validation

* chore: update to `4.4.2`

There was no conflict but updated the patch either way
@arcanis
Copy link
Member

arcanis commented Sep 3, 2021

Will this be backported to v2? I have a bunch of projects which still supports node 10

@SimenB yes, I'm in the process of cherry-picking it on 2.4; I'll have to afk, but I'll make sure it gets released tomorrow.

@SimenB
Copy link

SimenB commented Sep 3, 2021

Awesome, thanks! 🎉

@arcanis
Copy link
Member

arcanis commented Sep 6, 2021

Released 2.4.3

@SimenB
Copy link

SimenB commented Sep 6, 2021

Thanks! Can confirm it works 👍

@merceyz merceyz mentioned this pull request Sep 9, 2021
Closed
1 task
@mrienstra
Copy link
Contributor

These changes were released in 3.0.2: @yarnpkg/cli/3.0.2, @yarnpkg/pnp/3.0.2, @yarnpkg/plugin-pnp/3.0.2.
(Old news, just adding for the convenience of others)

@merceyz merceyz mentioned this pull request Nov 29, 2021
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arcanis arcanis Awaiting requested review from arcanis

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
5 participants
@merceyz @sushruth @SimenB @arcanis @mrienstra